🐛 Implement AppKitModalWalletListing.applicationId to correctly detect custom wallets on Android#334
Conversation
…ect custom wallets on Android
There was a problem hiding this comment.
Pull request overview
This pull request fixes a bug where custom wallets could not be detected on Android devices. The issue was that the app installation check only used the URI (mobile link) to verify if a wallet was installed, but on Android, the package name (application ID) is required for proper detection.
Changes:
- Added
applicationIdfield toAppKitModalWalletListingmodel to support Android package name - Updated
isInstalledmethod to accept and prioritizeapplicationIdparameter for Android app checks - Modified Android app check logic to try
applicationIdfirst before falling back to URI-based checks
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/reown_appkit/lib/modal/models/public/appkit_wallet_info.dart |
Added optional applicationId field to AppKitModalWalletListing model |
packages/reown_appkit/lib/modal/models/public/appkit_wallet_info.g.dart |
Generated JSON serialization code for new applicationId field |
packages/reown_appkit/lib/modal/models/public/appkit_wallet_info.freezed.dart |
Generated freezed code for new applicationId field including equality, hashCode, copyWith, and toString methods |
packages/reown_appkit/lib/modal/services/uri_service/i_url_utils.dart |
Updated interface to include applicationId parameter in isInstalled method |
packages/reown_appkit/lib/modal/services/uri_service/url_utils.dart |
Implemented Android-specific logic to check applicationId before URI for app installation detection |
packages/reown_appkit/lib/modal/services/explorer_service/explorer_service.dart |
Passed applicationId from wallet listing to isInstalled call for custom wallets |
Comments suppressed due to low confidence (1)
packages/reown_appkit/lib/modal/services/uri_service/url_utils.dart:132
- The current implementation checks both
isAppInstalledandisAppEnabledfor bothapplicationIdanduri. However, this could result in redundant checks whenapplicationIdanduriare the same value (for example, whenrdnsis used as the uri for Android wallets). Consider checking ifapplicationIdequalsurito avoid duplicate checks. This would improve performance and reduce unnecessary API calls to the AppCheck package.
Future<bool> _androidAppCheck(String uri, String? applicationId) async {
try {
bool installed = false;
if (applicationId != null && applicationId.isNotEmpty) {
installed = await AppCheck().isAppInstalled(applicationId);
}
if (installed) {
return true;
}
installed = await AppCheck().isAppInstalled(uri);
if (installed) {
return true;
}
bool enabled = false;
if (applicationId != null && applicationId.isNotEmpty) {
enabled = await AppCheck().isAppEnabled(applicationId);
}
if (enabled) {
return true;
}
enabled = await AppCheck().isAppEnabled(uri);
return enabled;
} catch (e) {
return false;
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -8,10 +8,15 @@ import 'package:reown_appkit/modal/services/explorer_service/models/redirect.dar | |||
|
|
|||
There was a problem hiding this comment.
Hey @AlexV525, thanks for the PR! The approach makes sense. A couple of suggestions:
1. Rename applicationId / application_id to androidAppId / android_app_id
The current name is generic and could be confused as applying to all platforms, but this is Android-specific (a package name for AppCheck). Renaming both the Dart field and JSON key makes the intent clear:
@JsonKey(name: 'android_app_id') String? androidAppId,2. Allow androidAppId alone to detect installed apps
Currently isInstalled returns false early if uri is null/empty (line 15-17), before ever reaching _androidAppCheck. This means a custom wallet configured with androidAppId but no mobileLink would never be detected on Android.
The early-return guards should be moved to be platform-specific so that on Android, androidAppId alone is sufficient:
@override
Future<bool> isInstalled(
String? uri, {
String? id,
String? androidAppId,
}) async {
if (PlatformUtils.canDetectInstalledApps()) {
final p = PlatformUtils.getPlatformExact();
try {
if (p == PlatformExact.android) {
return await _androidAppCheck(uri, androidAppId);
} else if (p == PlatformExact.ios && uri != null && uri.isNotEmpty && !uri.contains('wc://')) {
return await ReownCoreUtils.canOpenUrl(Uri.parse(uri).toString());
}
} on FormatException catch (e) {
if (id != null) {
_core.logger.i('[$runtimeType] $uri ($id): ${e.message}');
} else {
_core.logger.i('[$runtimeType] $uri: ${e.message}');
}
} catch (e) {
rethrow;
}
}
return false;
}And update _androidAppCheck to handle null uri:
Future<bool> _androidAppCheck(String? uri, String? androidAppId) async {
try {
if (androidAppId != null && androidAppId.isNotEmpty) {
if (await AppCheck().isAppInstalled(androidAppId) ||
await AppCheck().isAppEnabled(androidAppId)) {
return true;
}
}
if (uri == null || uri.isEmpty || uri.contains('wc://')) {
return false;
}
return await AppCheck().isAppInstalled(uri) ||
await AppCheck().isAppEnabled(uri);
} catch (e) {
return false;
}
}This way a custom wallet with only an androidAppId (no mobileLink) can still be detected on Android.
There was a problem hiding this comment.
Also, related to your other PR, we should replace uri.contains with startsWith
Description
Adding
AppKitModalWalletListing.applicationIdand checking it first when usingpackage:appcheckon Android.Resolves #333